Collect manifest information from multiple files, not just Bowerfile by thomashoneyman · Pull Request #240 · purescript/registry-dev

thomashoneyman · 2021-09-27T16:40:45Z

Closes #233, #220, #218. Supersedes #234.

This PR implements a LegacyImport script instead of a BowerImport script, because we now look at multiple files to try and import a manifest instead of just the Bowerfile. Those files include the LICENSE, package.json, bower.json, and spago.dhall and packages.dhall files.

Implementing the pipeline using multiple files to produce the manifest required shuffling around quite a bit, but the majority of the code moved into the constructManifestFields function which produces the license and dependencies for a package from a combination of the available files.

There are two other notable changes:

The withCache function works a little differently. First, it no longer requires EncodeJson and DecodeJson instances, because you can be storing something like a LICENSE file; it now uses String. Second, the withCache function now takes an ImportError -> Boolean function that allows you to cache files even if producing the file throws an exception. The exception is stored instead of the file contents, and when the file is read the exception is thrown. With this in place we can remove all the filterFailedPackages stuff, which is a bit error-prone, and we no longer need to read in the excluded packages.
The Error type has been changed around a little to accommodate the new files we're fetching. There's now a ResourceError that represents that files related to the package couldn't be fetched. MissingBowerfile has been replaced since it's not really an error any more, and NoDependencyFiles took its place to represent that both a spago and bower file are missing.

…e in BowerImport

thomashoneyman · 2021-09-28T22:34:48Z

These are the results, working from a .cache with only releases cached:

Number of successful packages: 1448
Number of failed packages: 375
Number of successful versions: 10234
Number of failed versions: 1281
Failures by error:
  manifestError: 1214 occurrences (314 packages / 1214 versions)
    badLicense: 480 occurrences (76 packages / 480 versions)
    missingLicense: 452 occurrences (141 packages / 452 versions)
    badDependencyVersions: 307 occurrences (114 packages / 307 versions)
    badVersion: 50 occurrences (34 packages / 50 versions)
  noDependencyFiles: 66 occurrences (34 packages / 66 versions)
  resourceError: 32 occurrences (32 packages / 0 versions)
  malformedPackageName: 1 occurrences (1 packages / 1 versions)

It's a significant improvement! Compare that to the current main branch:

Number of successful packages: 1369
Number of failed packages: 474
Number of successful versions: 9565
Number of failed versions: 1694
Failures by error:
  manifestError: 1378 occurrences (371 packages / 1378 versions)
    missingLicense: 928 occurrences (241 packages / 928 versions)
    badDependencyVersions: 283 occurrences (104 packages / 283 versions)
    badLicense: 179 occurrences (45 packages / 179 versions)
    badVersion: 48 occurrences (32 packages / 48 versions)
  nonRegistryDependencies: 246 occurrences (47 packages / 246 versions)
  missingBowerfile: 64 occurrences (34 packages / 64 versions)
  noReleases: 32 occurrences (32 packages / 0 versions)
  malformedBowerJson: 5 occurrences (4 packages / 5 versions)
  malformedPackageName: 1 occurrences (1 packages / 1 versions)

The number of missing licenses has been cut in half, and about 700 more versions are included in the final registry. I would like to keep looking at the exclusions file to see if any packages are suspiciously failing, but on the whole this is a nice improvement over the current stats.

thomashoneyman · 2021-09-28T23:12:26Z

The tests are failing because I added a Licensee test; we’re going to have to add that to CI as a CLI tool.

thomashoneyman · 2021-09-30T16:49:10Z

Whew, sorry, had to do a little CI fixup (case sensitivity is a bear!), but this is now actually good to go.

f-f

Great work @thomashoneyman! I left a bunch of small comments, and this is almost ready to go

Co-authored-by: Fabrizio Ferrai <fabrizio.ferrai@gmail.com>

thomashoneyman · 2021-09-30T17:43:48Z

Some of my responses to your comments are only showing up in the review, and not in this main PR view, so you might have to read them in the review tab. Thanks for your comments! I believe I've addressed everything.

f-f

Thanks for addressing the feedback! I left a few minor comments, and I'll approve this since it's basically good to go 🙂

f-f · 2021-10-01T00:17:06Z

+        state <- AVar.take var
+        AVar.put (modify state) var


Great! Then since we repeat this pattern many times I was wondering if it'd be worth making a helper that takes, does stuff, and then puts, so that we reduce clutter. No strong opinion on this though, feel free to ignore

f-f · 2021-10-01T00:20:20Z

        }

-    res <- API.readOperation "test/fixtures/issue_comment.json"
+    res <- API.readOperation "test/Fixtures/issue_comment.json"


Oh, I didn't realize then - thanks for the pointer. I have a slight preference for the lowercase version (I am definitely traumatized by macOS' filesystem shenanigans with that) so I'd appreciate if you can switch to that, but no worries if you wouldn't like to

Co-authored-by: Fabrizio Ferrai <fabrizio.ferrai@gmail.com>

thomashoneyman added 8 commits September 23, 2021 11:01

Rename Bower import -> legacy import

8b512bb

Remove jsonic wrapping error, add fileRequest function

27c2cf6

Add most of constructRawManifest

e4074bb

fixup toManifest function

7147e08

Merge bowerfile and manifest fetches

ff68bb3

Add some FIXME comments

7efd81c

Move manifest fields to dedicated file

237221e

Clean up package filtering and re-enable dependency check

318a6ee

thomashoneyman added the CI:bower-import label Sep 27, 2021

Undo rename BowerImport -> LegacyImport

3967ac3

This was linked to issues Sep 28, 2021

Fall back to spago.dhall files for packages without a bower.json #218

Closed

Parse LICENSE file if no license is listed in the Bowerfile #220

Closed

Add licensee tests, add parallel processing, fix license parsing issu…

f8b676a

…e in BowerImport

thomashoneyman commented Sep 28, 2021

View reviewed changes

thomashoneyman added 3 commits September 30, 2021 14:05

Use ci directory nix-shell in CI

ab4dfb3

fixtures -> Fixtures

69e85ff

missed one

21a8801

thomashoneyman requested a review from f-f September 30, 2021 14:01

Move fixtures dir to Fixtures

d4b5193

f-f reviewed Sep 30, 2021

View reviewed changes

thomashoneyman and others added 3 commits September 30, 2021 19:29

Update ci/src/Registry/Scripts/BowerImport.purs

990e52b

Co-authored-by: Fabrizio Ferrai <fabrizio.ferrai@gmail.com>

Update ci/src/Registry/Scripts/BowerImport/SpagoJson.purs

220cd68

rename Bowerfile -> BowerFile

f9da2e7

f-f approved these changes Oct 1, 2021

View reviewed changes

thomashoneyman and others added 2 commits October 1, 2021 13:30

Update ci/spago.dhall

efacbed

Co-authored-by: Fabrizio Ferrai <fabrizio.ferrai@gmail.com>

Address remaining feedback

7c60aad

thomashoneyman merged commit f4c32fb into master Oct 4, 2021

thomashoneyman deleted the trh/fallback branch October 4, 2021 12:47

This was referenced Oct 4, 2021

Fall back to spago files if there is no bower file in a repository #234

Closed

Rename Bower Import script to Legacy Import #241

Merged

Conversation

thomashoneyman commented Sep 27, 2021

Uh oh!

thomashoneyman commented Sep 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomashoneyman commented Sep 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomashoneyman commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f-f left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomashoneyman commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f-f left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

f-f Oct 1, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

f-f Oct 1, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thomashoneyman commented Sep 28, 2021 •

edited

Loading

thomashoneyman commented Sep 30, 2021 •

edited

Loading

thomashoneyman commented Sep 30, 2021 •

edited

Loading

f-f left a comment •

edited

Loading